Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

###Feature/Add extractors and test samples for SQLException #1779

Conversation

BereKanters
Copy link
Contributor

  • Introduced feature extractors for SQLException:

    • errorCode: Extracts the error code from an SQLException instance.

    • sqlState: Extracts the SQL state from an SQLException instance.

    • Added test samples to validate the new feature extractors:

    • errorCodeFeature: Tests the extraction and validation of the error code.

    • sqlStateFeature: Tests the extraction and validation of the SQL state.

    • errorCode: Tests grouped assertions for the error code.

    • sqlState: Tests grouped assertions for the SQL state.

    Note: I tried to work on the KSP Plugin, seeing if I could get it to be autogenerated but ill be needing more help with that.


I confirm that I have read the Contributor Agreements v1.0, agree to be bound on them and confirm that my contribution is compliant.

…ndows workflows

- Added a new CI workflow using vlsi/github-actions-random-matrix to generate a dynamic matrix for testing on both Ubuntu and Windows with JDK 8 and JDK 11.
- Merged existing build steps for Ubuntu and Windows into a single workflow file (ci.yml).
- Moved common steps, such as caching the Android jar and compiling, into a separate setup job to improve CI efficiency.
- Updated README.md to reflect the new combined CI workflow badge.
- Improved test coverage and reduced CI resource usage by leveraging the matrix strategy.
…d Windows workflows updated based on feedback

- Added custom matrix generation scripts (`matrix_builder.js` and `matrix.js`) to dynamically generate the matrix for CI jobs.
- Configured the GitHub Actions workflow (`ci.yml`) to use the custom matrix generator.
- Renamed the `setup` job to `dexer` and included the step to check if Atrium's -jvm.jar can be dexed.
- Introduced feature extractors for SQLException:
  - errorCode: Extracts the error code from an SQLException instance.
  - sqlState: Extracts the SQL state from an SQLException instance.

  - Added test samples to validate the new feature extractors:
    - errorCodeFeature: Tests the extraction and validation of the error code.
    - sqlStateFeature: Tests the extraction and validation of the SQL state.
    - errorCode: Tests grouped assertions for the error code.
    - sqlState: Tests grouped assertions for the SQL state.
@BereKanters BereKanters requested a review from robstoll as a code owner June 14, 2024 23:15
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/matrix.js Outdated Show resolved Hide resolved
.github/workflows/matrix_builder.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines 21 to 27
fails {
expect(exception)
.errorCode // subject is now of type Int
.toEqual(9999) // fails
.toBeGreaterThan(1000) // not evaluated/reported because `toEqual` already fails
// use `.errorCode { ... }` if you want all assertions evaluated
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest you align the comments along the line of:

Suggested change
fails {
expect(exception)
.errorCode // subject is now of type Int
.toEqual(9999) // fails
.toBeGreaterThan(1000) // not evaluated/reported because `toEqual` already fails
// use `.errorCode { ... }` if you want all assertions evaluated
}
fails {
expect(exception)
.errorCode // subject is now of type Int
.toEqual(9999) // fails
.toBeGreaterThan(1000) // not evaluated/reported because `toEqual` already fails
// use `.errorCode { ... }` if you want all assertions evaluated
}

Comment on lines 47 to 62
@Test
fun errorCode() {
val exception = SQLException("Test exception", "42000", 1001)

expect(exception).errorCode { // subject within this expectation-group is of type Int
toBeGreaterThan(1000)
toEqual(1001)
} // subject here is back to type SQLException

fails {
expect(exception).errorCode {
toEqual(9999) // fails
toBeGreaterThan(1000) // still evaluated even though `toEqual` already fails
}
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move up right after errorCodeFeature

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

@robstoll
Copy link
Owner

Note: I tried to work on the KSP Plugin, seeing if I could get it to be autogenerated but ill be needing more help with that.

Nice, I suggest that we postpone that one to a next PR though. I am not convinced we need a KSP plugin as we might want to operate on code we don't actually compile (as in this case, the SQLException was already compiled).

- Added KDoc comments with @SInCE 1.3.0 and links to the sample file.
- Changed file naming to `sqlExceptionFeatureExtractors` and `SQLExceptionFeaturesSamples`
- Removed accidentally commited files
- Aligned comments and ordered fun
@BereKanters BereKanters requested a review from robstoll June 15, 2024 23:35
BereKanters and others added 12 commits June 16, 2024 17:50
…ndows workflows

- Added a new CI workflow using vlsi/github-actions-random-matrix to generate a dynamic matrix for testing on both Ubuntu and Windows with JDK 8 and JDK 11.
- Merged existing build steps for Ubuntu and Windows into a single workflow file (ci.yml).
- Moved common steps, such as caching the Android jar and compiling, into a separate setup job to improve CI efficiency.
- Updated README.md to reflect the new combined CI workflow badge.
- Improved test coverage and reduced CI resource usage by leveraging the matrix strategy.
…d Windows workflows updated based on feedback

- Added custom matrix generation scripts (`matrix_builder.js` and `matrix.js`) to dynamically generate the matrix for CI jobs.
- Configured the GitHub Actions workflow (`ci.yml`) to use the custom matrix generator.
- Renamed the `setup` job to `dexer` and included the step to check if Atrium's -jvm.jar can be dexed.
- Introduced feature extractors for SQLException:
  - errorCode: Extracts the error code from an SQLException instance.
  - sqlState: Extracts the SQL state from an SQLException instance.

  - Added test samples to validate the new feature extractors:
    - errorCodeFeature: Tests the extraction and validation of the error code.
    - sqlStateFeature: Tests the extraction and validation of the SQL state.
    - errorCode: Tests grouped assertions for the error code.
    - sqlState: Tests grouped assertions for the SQL state.
- Added KDoc comments with @SInCE 1.3.0 and links to the sample file.
- Changed file naming to `sqlExceptionFeatureExtractors` and `SQLExceptionFeaturesSamples`
- Removed accidentally commited files
- Aligned comments and ordered fun
…s Workflows (robstoll#1776)

* ###feature/Integrate random matrix generation and merge Ubuntu and Windows workflows

- Added a new CI workflow using vlsi/github-actions-random-matrix to generate a dynamic matrix for testing on both Ubuntu and Windows with JDK 11 and JDK 17
- Merged existing build steps for Ubuntu and Windows into a single workflow file.
- Removed comment from constructor
- Added comment to constructor
- Added this.namePattern = [];
- Introduced feature extractors for SQLException:
  - errorCode: Extracts the error code from an SQLException instance.
  - sqlState: Extracts the SQL state from an SQLException instance.
Copy link

codecov bot commented Jun 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.74%. Comparing base (9c560e9) to head (552f710).
Report is 44 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #1779   +/-   ##
=========================================
  Coverage     92.73%   92.74%           
  Complexity      116      116           
=========================================
  Files           442      443    +1     
  Lines          4941     4945    +4     
  Branches        234      234           
=========================================
+ Hits           4582     4586    +4     
  Misses          312      312           
  Partials         47       47           
Flag Coverage Δ
current 92.46% <100.00%> (+<0.01%) ⬆️
current_windows 91.50% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

import java.sql.SQLException

/**
* Extracts the [errorCode][SQLException.getErrorCode] property from the subject of the assertion.
Copy link
Owner

@robstoll robstoll Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please copy the kdoc from collectionFeatureExtractors.kt -> val size and adopt. I prefer if they are all the same, this way changes in the future can be made via search and replace.

get() = feature("errorCode", SQLException::getErrorCode)

/**
* Extracts the [errorCode][SQLException.getErrorCode] property from the subject of the assertion and makes it the new subject.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here, please copy the KDoc from collectionFeatureExtractors.kt -> fun size

@@ -518,6 +518,13 @@ public final class ch/tutteli/atrium/api/fluent/en_GB/SequenceSubjectChangersKt
public static final fun asList (Lch/tutteli/atrium/creating/Expect;Lkotlin/jvm/functions/Function1;)Lch/tutteli/atrium/creating/Expect;
}

public final class ch/tutteli/atrium/api/fluent/en_GB/SqlExceptionFeatureExtractorsKt {
Copy link
Owner

@robstoll robstoll Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you also need to generate the api for Kotlin 1.9 and newer. Run KOTLIN_VERSION=1.9 ./gradlew apiDump to achieve this (or copy those lines over to using-kotlin-1.9-or-newer/atrium-api-fluent.api).

@robstoll
Copy link
Owner

@BereKanters please let me know in case you need help

@robstoll
Copy link
Owner

@hubtanaka would you like to take this one over? I guess @BereKanters doesn't have time to finish it.

@hubtanaka
Copy link
Collaborator

@robstoll Yes. I'll work on this.

@hubtanaka
Copy link
Collaborator

@robstoll
I created new PR for this issue.
#1820

by the way, I didn't find out how to commit here directly because I'm not familiar with GitHub yet.
So I wonder that this is a correct way to do on GitHub.
please let me know if there is more effective something to take over and proceed this PR.

@robstoll
Copy link
Owner

Creating a new PR was the correct why as you would need to have access to Berekaners repository if you wanted to commit here directly. Closing this in favour of the new PR

@robstoll robstoll closed this Aug 23, 2024
@hubtanaka
Copy link
Collaborator

Thank you so much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants